Skip to content

feat(deno): redis diagnostics channel based integration for deno#21087

Merged
isaacs merged 5 commits into
developfrom
isaacs/deno-redis-dc
May 29, 2026
Merged

feat(deno): redis diagnostics channel based integration for deno#21087
isaacs merged 5 commits into
developfrom
isaacs/deno-redis-dc

Conversation

@isaacs
Copy link
Copy Markdown
Member

@isaacs isaacs commented May 20, 2026

Refactor the redis and ioredis diagnostics_channel integration logic into core/src/integrations, and create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration adds _sentrySpan onto the data in a RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test covering both redis clients.

fix: #21221
fix: JS-2630

@isaacs isaacs requested a review from a team as a code owner May 20, 2026 21:20
@isaacs isaacs requested review from andreiborza and mydea and removed request for a team May 20, 2026 21:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

size-limit report 📦

Path Size % Change Change
@sentry/browser 27.17 kB - -
@sentry/browser - with treeshaking flags 25.62 kB - -
@sentry/browser (incl. Tracing) 45.25 kB - -
@sentry/browser (incl. Tracing + Span Streaming) 47.47 kB - -
@sentry/browser (incl. Tracing, Profiling) 50.22 kB - -
@sentry/browser (incl. Tracing, Replay) 84.83 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 74.39 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 89.53 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 102.15 kB - -
@sentry/browser (incl. Feedback) 44.34 kB - -
@sentry/browser (incl. sendFeedback) 31.98 kB - -
@sentry/browser (incl. FeedbackAsync) 37.08 kB - -
@sentry/browser (incl. Metrics) 28.25 kB - -
@sentry/browser (incl. Logs) 28.48 kB - -
@sentry/browser (incl. Metrics & Logs) 29.19 kB - -
@sentry/react 28.99 kB - -
@sentry/react (incl. Tracing) 47.51 kB - -
@sentry/vue 32.2 kB - -
@sentry/vue (incl. Tracing) 47.16 kB - -
@sentry/svelte 27.19 kB - -
CDN Bundle 29.55 kB - -
CDN Bundle (incl. Tracing) 47.81 kB - -
CDN Bundle (incl. Logs, Metrics) 31.05 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) 49.04 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) 70.31 kB - -
CDN Bundle (incl. Tracing, Replay) 85.18 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 86.35 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 91.05 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 92.21 kB - -
CDN Bundle - uncompressed 87.59 kB - -
CDN Bundle (incl. Tracing) - uncompressed 144.09 kB - -
CDN Bundle (incl. Logs, Metrics) - uncompressed 92.08 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 147.85 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 216.81 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 262.87 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 266.61 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 276.57 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 280.3 kB - -
@sentry/nextjs (client) 50.03 kB - -
@sentry/sveltekit (client) 45.68 kB - -
@sentry/core/server 76.56 kB +0.83% +628 B 🔺
@sentry/core/browser 63.09 kB - -
@sentry/node-core 61.69 kB - -
@sentry/node 130.51 kB +0.09% +112 B 🔺
@sentry/node - without tracing 74.1 kB - -
@sentry/aws-serverless 86.29 kB - -
@sentry/cloudflare (withSentry) - minified 171.5 kB - -
@sentry/cloudflare (withSentry) 429.29 kB - -

View base workflow run

isaacs added a commit that referenced this pull request May 20, 2026
)

Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test.
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from f176fd8 to 0fc320c Compare May 20, 2026 22:13
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from 1048f32 to 72be1fa Compare May 20, 2026 22:18
Comment thread .github/workflows/build.yml
isaacs added a commit that referenced this pull request May 21, 2026
isaacs added a commit that referenced this pull request May 21, 2026
isaacs added a commit that referenced this pull request May 21, 2026
)

Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test.
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from 11525d8 to a8d3cb7 Compare May 21, 2026 17:31
Comment thread packages/deno/src/integrations/redis.ts Outdated
Comment thread packages/core/src/integrations/redis/redis-statement-serializer.ts Outdated
Comment thread packages/core/src/integrations/redis/redis-dc-subscriber.ts
Comment thread packages/node/src/integrations/tracing/redis/index.ts
isaacs added a commit that referenced this pull request May 21, 2026
Comment thread packages/node/src/utils/redisCache.ts
Comment thread packages/deno/src/sdk.ts Outdated
isaacs added a commit that referenced this pull request May 21, 2026
isaacs added a commit that referenced this pull request May 21, 2026
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from 8226aaf to 5863e6c Compare May 21, 2026 21:54
@isaacs isaacs enabled auto-merge (squash) May 21, 2026 23:10
isaacs added a commit that referenced this pull request May 22, 2026
)

Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test.
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from 5863e6c to cd298fa Compare May 22, 2026 17:01
Comment thread packages/core/src/integrations/redis/redis-statement-serializer.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

👋 @mydea, @andreiborza — Please review this PR when you get a chance!

isaacs added a commit that referenced this pull request May 25, 2026
)

Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test.
@JPeer264 JPeer264 self-requested a review May 27, 2026 09:52
isaacs added a commit that referenced this pull request May 27, 2026
)

Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test.
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from e46685b to 1f6c601 Compare May 27, 2026 18:47
Comment thread packages/core/src/integrations/redis/redis-statement-serializer.ts Outdated
Copy link
Copy Markdown
Member

@JPeer264 JPeer264 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just have one m which I want to get sorted before approving

Comment thread packages/core/src/integrations/redis/redis-dc-subscriber.ts Outdated
Comment thread packages/core/src/integrations/redis/redis-dc-subscriber.ts
Comment thread packages/core/src/integrations/redis/redis-dc-subscriber.ts
Comment thread packages/core/src/server-exports.ts
@github-actions
Copy link
Copy Markdown
Contributor

👋 @mydea, @andreiborza — Please review this PR when you get a chance!

@mydea mydea requested review from logaretm and nicohrubec and removed request for mydea May 28, 2026 12:35
Copy link
Copy Markdown
Member

@logaretm logaretm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do u think we can merge this first

#21187

since it will also allow this PR to do it for both ioredis and redis clients.

@JPeer264
Copy link
Copy Markdown
Member

Do u think we can merge this first

I'm personally fine with this. Approved the other PR, waiting for @isaacs decision.

isaacs added a commit that referenced this pull request May 28, 2026
)

Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test.
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from 1f6c601 to 6a2f9b6 Compare May 28, 2026 18:29
)

Refactor the redis and ioredis diagnostics_channel integration logic
into core/src/integrations, and create a Deno integration that uses the
same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test covering both redis clients.

fix: #21221
fix: JS-2630
@isaacs isaacs force-pushed the isaacs/deno-redis-dc branch from 6a2f9b6 to ebbbc65 Compare May 29, 2026 01:55
@isaacs isaacs requested review from JPeer264 and logaretm May 29, 2026 01:55
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ebbbc65. Configure here.

Comment thread packages/core/src/integrations/redis/redis-dc-subscriber.ts Outdated
Copy link
Copy Markdown
Member

@JPeer264 JPeer264 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Lint is failing, but it looks like a small change.

): void {
const channel = tracingChannel<T>(channelName, data => {
const args = getCommandArgs(data);
const statement = safeSerialize(data.command, args);
Copy link
Copy Markdown
Member

@logaretm logaretm May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: tracing channels on redis already sanitize the command statements or is that a different thing?

https://github.com/redis/ioredis/blob/main/lib/tracing.ts#L10-L22

https://github.com/logaretm/node-redis/blob/master/packages/client/lib/client/tracing.ts#L39-L44

It was based off the one implemented by OTEL originally, and Redis team wanted Redis to own that part.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, good catch. Yes, this was ported in from the otel implementation, and looks like we missed it the first time around, but it's just spinning extra CPU cycles unnecessarily, since the dc publishers already do this for us.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also means we don't have to handle Uint8Array/Buffer args, which simplifies it even further, actually.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@logaretm Ended up being a pretty big diff. Care to take another look at that bit? 489eee9

@isaacs isaacs disabled auto-merge May 29, 2026 14:39
@isaacs isaacs merged commit 18fbbb7 into develop May 29, 2026
266 of 267 checks passed
@isaacs isaacs deleted the isaacs/deno-redis-dc branch May 29, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(deno): redis diagnostics channel based integration for deno

3 participants